Why create our own struct instead of using stats.QuerySamples and passing that to the appropriate operators?

It would also make it easier to implement TotalSamplesPerStep if we want to support that too (which I think we do)

Why create our own struct instead of using stats.QuerySamples and passing that to the appropriate operators?

Given we don't support anything other than TotalSamples in MQE, I wanted to make this clear in the code by using a struct that only had a field for TotalSamples.

It would also make it easier to implement TotalSamplesPerStep if we want to support that too (which I think we do)

I don't think we want to do this unless there's a specific need for it - per-step stats are considered experimental and disabled by default in Prometheus, and are not possible to enable in Mimir as far as I can see. The docs for this also state that the value for each step should be the same as if the query was run as an instant query, so anyone who wanted this information could run the query as an instant query for the step(s) they're interested in.

Why create our own struct instead of using stats.QuerySamples and passing that to the appropriate operators?

Given we don't support anything other than TotalSamples in MQE, I wanted to make this clear in the code by using a struct that only had a field for TotalSamples.

I would be happy with a comment in query.go, but I'm not opposed to a separate struct.

It would also make it easier to implement TotalSamplesPerStep if we want to support that too (which I think we do)

I don't think we want to do this unless there's a specific need for it - per-step stats are considered experimental and disabled by default in Prometheus, and are not possible to enable in Mimir as far as I can see. The docs for this also state that the value for each step should be the same as if the query was run as an instant query, so anyone who wanted this information could run the query as an instant query for the step(s) they're interested in.

Fair enough, but also thinking of any future stats. I don't mind a separate struct though.

We can also compare the samples loaded as part of our test gauntlet if we expect it to be the same in all cases

This is currently difficult due to the optimisation in prometheus/prometheus#14097, as Prometheus' engine sometimes skips loading data for histograms if it's not needed. MQE does not yet have the same optimisation, so there are some expected differences in the total sample count from the two engines in some cases.

Given the tests in TestQueryStats, and the fact the statistics are informational and may differ between engines in the future due to other optimisations, I'm tempted to leave this as-is.

Thoughts?

Happy to leave it out if that's the case.

It might be interesting to see some much larger queries/time ranges etc to see if we are returning consistent results. We could perhaps use the same data generated from the benchmarks to create some large queries (of just floats since NH will be different). Then have a flag to RequireEqualResults to compare them etc.

This isn't a blocker.